Skip to content

[Store] Clarify cache stats semantics - #2248

Merged
yokinoshitayoki merged 1 commit into
kvcache-ai:mainfrom
CAICAIIs:issue-972-store-cache-stats
May 28, 2026
Merged

yokinoshitayoki merged 1 commit into
kvcache-ai:mainfrom
CAICAIIs:issue-972-store-cache-stats

Conversation

@CAICAIIs

@CAICAIIs CAICAIIs commented May 28, 2026

Copy link
Copy Markdown
Contributor

Description

Refs #972.

This PR clarifies the semantics of Mooncake Store cache stats.

The existing *_HIT_RATE values in calculate_cache_stats() are Store-observed reuse signals. They divide cumulative Store-side hits by the current cached object count, so they may exceed 1.0. They are not end-to-end request-level or token-level cache hit ratios.

This change:

  • Adds clearer enum aliases such as *_HITS_PER_CURRENT_CACHED_OBJECT and *_CURRENT_CACHED_OBJECTS.
  • Keeps the existing *_HIT_RATE, *_TOTAL, and VALID_GET_RATE enum values for RPC/API compatibility.
  • Updates comments and metric descriptions to avoid presenting Store-side stats as final request/token-level hit ratios.
  • Documents that request-level hit ratio should be calculated by Conductor or the inference engine.
  • Adds test coverage for enum value compatibility, alias equivalence, isolated CalcCacheStatsTest execution, and values greater than 1.0.
  • Adds the missing transport-selector TENT toctree entry so the strict Sphinx docs build can include the existing page.

Module

  • Transfer Engine (mooncake-transfer-engine)
  • Mooncake Store (mooncake-store)
  • Mooncake EP (mooncake-ep)
  • Integration (mooncake-integration)
  • P2P Store (mooncake-p2p-store)
  • Python Wheel (mooncake-wheel)
  • PyTorch Backend (mooncake-pg)
  • Mooncake RL (mooncake-rl)
  • CI/CD
  • Docs
  • Other

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Breaking change
  • Documentation update
  • Other

How Has This Been Tested?

On a remote Linux validation host:

  • git diff --check HEAD~1
  • ./scripts/code_format.sh --check --base HEAD~1
  • cd docs && make html SPHINXOPTS=-W SPHINXBUILD=/root/miniconda3/bin/sphinx-build
  • cmake --build build --target master_metrics_test -j"$(nproc)"
  • ./build/mooncake-store/tests/master_metrics_test --gtest_filter=MasterMetricsTest.CalcCacheStatsTest
  • MC_METADATA_SERVER=http://127.0.0.1:8080/metadata DEFAULT_KV_LEASE_TTL=500 ctest --test-dir build -R master_metrics_test --output-on-failure

Checklist

  • I have performed a self-review of my own code.
  • I have formatted my own code using ./scripts/code_format.sh before submitting.
  • I have updated the documentation.
  • I have added tests to prove my changes are effective.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the cache hit metrics in Mooncake Store to clarify that they represent Store-observed cache reuse signals rather than end-to-end request-level or token-level hit ratios. It introduces enum aliases (such as MEMORY_CURRENT_CACHED_OBJECTS and MEMORY_HITS_PER_CURRENT_CACHED_OBJECT) to replace legacy hit rate names, updates documentation, and adds corresponding unit tests. Feedback highlights two main issues: first, moving VALID_GET_RATE in the CacheHitStat enum alters the underlying integer values, which breaks RPC backward compatibility; second, the new unit test CalcCacheStatsTest is not hermetic because it relies on the residual state of the MasterMetricManager singleton from previous tests.

Comment thread mooncake-store/include/master_metric_manager.h Outdated
Comment thread mooncake-store/tests/master_metrics_test.cpp Outdated
@CAICAIIs
CAICAIIs marked this pull request as draft May 28, 2026 04:40
Signed-off-by: CAICAIIs <3360776475@qq.com>
@CAICAIIs
CAICAIIs force-pushed the issue-972-store-cache-stats branch from 931a0e8 to 6213188 Compare May 28, 2026 05:50
@CAICAIIs
CAICAIIs marked this pull request as ready for review May 28, 2026 05:54
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.31818% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
mooncake-store/src/master_metric_manager.cpp 73.68% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@yokinoshitayoki

Copy link
Copy Markdown
Collaborator

LGTM. Thanks a lot for your clarification!

@yokinoshitayoki
yokinoshitayoki merged commit f6b4adb into kvcache-ai:main May 28, 2026
33 of 38 checks passed
A-Liuhao pushed a commit to A-Liuhao/Mooncake that referenced this pull request Jun 25, 2026
Signed-off-by: CAICAIIs <3360776475@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants